-
-
Notifications
You must be signed in to change notification settings - Fork 808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add tests for IEEE 754-2019 compliance #6571
Conversation
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
@anandkaranubc Per failing CI checks, looks like our |
Oops, there is a bug. Mainly because of no type casting on stdlib/lib/node_modules/@stdlib/math/base/special/log2/lib/main.js Lines 118 to 126 in 7966727
But in C, Typecasting it back again in stdlib/lib/node_modules/@stdlib/math/base/special/log2/src/main.c Lines 71 to 79 in 7966727
cc: @Planeshifter @kgryte |
Signed-off-by: Karan Anand <[email protected]>
@anandkaranubc For L22, if the condition assumes if ( (hx|0) < HIGH_MIN_NORMAL_EXP ) {
// Case: x < 2**-1022
if ( ( (hx&ABS_MASK) | lx ) === 0 ) {
return NINF;
} |
@kgryte Thanks for the reply! Can I do |
PR-URL: stdlib-js#6599 Ref: stdlib-js#365 Reviewed-by: Philipp Burckhardt <[email protected]>
PR-URL: stdlib-js#6590 Reviewed-by: Philipp Burckhardt <[email protected]> Signed-off-by: stdlib-bot <[email protected]>
PR-URL: stdlib-js#6595 Ref: stdlib-js#365 Reviewed-by: Philipp Burckhardt <[email protected]>
PR-URL: stdlib-js#6591 Reviewed-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6617 Reviewed-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6612 Reviewed-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6611 Reviewed-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6607 Reviewed-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6608 Co-authored-by: Athan Reines <[email protected]> Reviewed-by: Athan Reines <[email protected]> Signed-off-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6587 Reviewed-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6596 Closes: stdlib-js#6588 Co-authored-by: Athan Reines <[email protected]> Reviewed-by: Athan Reines <[email protected]> Signed-off-by: Mahfuza Humayra Mohona <[email protected]> Signed-off-by: Athan Reines <[email protected]>
PR-URL: stdlib-js#6597 Ref: stdlib-js#365 Reviewed-by: Athan Reines <[email protected]> Signed-off-by: Karan Anand <[email protected]>
PR-URL: stdlib-js#5639 Closes: stdlib-js/metr-issue-tracker#20 Co-authored-by: Athan Reines <[email protected]> Reviewed-by: Athan Reines <[email protected]> Signed-off-by: Muhammad Haris <[email protected]> Signed-off-by: Athan Reines <[email protected]>
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
@anandkaranubc Actually, are we sure the issue is with
|
/stdlib merge |
@anandkaranubc Ah, I see. The special case is
|
Coverage Report
The above coverage report was generated for the changes in this PR. |
@@ -132,7 +132,7 @@ function log10( x ) { | |||
|
|||
// Subnormal number, scale up x: | |||
x *= TWO54; | |||
hx = getHighWord( x ); | |||
hx = getHighWord( x ) | 0; // asm type annotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also updated this line accordingly to be consistent with upstream (and C) implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our C implementation:
stdlib/lib/node_modules/@stdlib/math/base/special/log10/src/main.c
Lines 104 to 106 in 864fc4f
xc *= TWO54; | |
stdlib_base_float64_get_high_word( xc, &hx ); | |
ihx = (int32_t)hx; |
The failing test is unrelated to this PR. |
@anandkaranubc I'll fix that up on |
/stdlib merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Progresses #365
Description
This pull request:
log*
Related Issues
This pull request:
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers